stylix: allow choosing testbed desktop by mateusauler · Pull Request #1222 · nix-community/stylix

mateusauler · 2025-05-04T15:57:02Z

This adds stylix.testbed.ui.desktop which allows the testbed to pick what desktop environment/window manager will be ran inside the VM.

This PR is currently a PoC and I'm open to any suggestions. I'm actively working on it, so everything is subject to change.

Relevant: #1192 (comment)

Things done

Notify maintainers

@danth @trueNAHO @awwpotato @Flameopathic

Who else?

danth · 2025-05-04T21:30:02Z

This seems a bit overkill IMO. The standard configuration was meant as a default for when the target is an application rather than a DE/WM itself - with the plan that GNOME would eventually be replaced with something more minimal like cage. If testbeds are targeting a DE/WM, or an application which requires something more specific then they should include their own configuration from scratch.

By adding options like this, we risk duplicating a lot of the services.xserver.* options from NixOS as essentially aliases which just enable a couple of things.

0xda157 · 2025-05-04T21:33:06Z

something more minimal like cage

imo cage is too minimal because it doesn't interact well with popups iirc. perhaps we use a minimal wm like river.

mateusauler · 2025-05-04T22:25:05Z

The reasoning is that:

a) Some applications might be desktop specific, like gnome-text-editor for example. So if we support multiple applications for the same desktop, we end up with a lot of duplicate code and potentially inconsistent setups. If a module needs a more specific setup on top of the desktop, it can handle that.

b) In the case of mako (#1192), for example, I had to replicate basically all of Hyprland's testbed code.
Some other modules we already support that could have this problem are: dunst, hyprlock, hyprpaper, rofi, swaylock, waybar, wpaperd, xresources.

We could limit our selection to a couple desktops for their specific properties (Eg. X11/Wayland, doesn't include a bar, doesn't handle notifications). Right now, I added all desktops we have testbeds for.

Flameopathic

When adding testbeds, I was running into the issue of needing different WMs to test different targets. I think this would be a great addition; thank you!

trueNAHO

This seems a bit overkill IMO. The standard configuration was meant as a default for when the target is an application rather than a DE/WM itself - with the plan that GNOME would eventually be replaced with something more minimal like cage.

Yes, migrating to the minimal functioning graphical environment would be the ideal solution.

If testbeds are targeting a DE/WM, or an application which requires something more specific then they should include their own configuration from scratch.

I agree that providing interfaces to several graphical environments is arguably overkill. The main benefit of this abstraction is the reduction of the very occasional code duplication:

a) Some applications might be desktop specific, like gnome-text-editor for example. So if we support multiple applications for the same desktop, we end up with a lot of duplicate code and potentially inconsistent setups. If a module needs a more specific setup on top of the desktop, it can handle that.

By adding options like this, we risk duplicating a lot of the services.xserver.* options from NixOS as essentially aliases which just enable a couple of things.

Yes, but providing a graphical environment interface increases the consistency for the same graphical environment across testbeds:

b) In the case of mako (#1192), for example, I had to replicate basically all of Hyprland's testbed code.

Despite this abstraction being arguably overkill, it allows testbeds to target X11 and Wayland individually. However, does XWayland nowadays not cover all our X11 use cases:

We could limit our selection to a couple desktops for their specific properties (Eg. X11/Wayland, doesn't include a bar, doesn't handle notifications).

This solves the problem generally by interfacing all our supported graphical environments:

Right now, I added all desktops we have testbeds for.

However, I agree with danth's stance of this abstraction being overkill. The best solution would be to replace GNOME with something like cage and make everything work there.

Instead of #1192 changing the entire graphical environment, it would be better if it would somehow be able to replace GNOME's notification daemon:

https://github.com/danth/stylix/blob/b66b220fb02cd0713eb93d82dcf36962d04b0d86/modules/mako/testbeds/default.nix#L4

Regardless of this PR being merged, the change in /modules/bspwm/testbeds/default.nix of replacing xsession.windowManager.bspwm.startupPrograms with stylix.testbed.ui.command.text is probably good to keep since it uses our consistent stylix.testbed.ui interface.

nukdokplex · 2025-05-11T22:46:26Z

+1 wayprompt doesn't work in gnome environment for some reason, i tested it on sway and hyprland, works great on those wms

panchoh · 2025-05-12T09:10:20Z

+1 wayprompt doesn't work in gnome environment for some reason, i tested it on sway and hyprland, works great on those wms

Ah, the reason is that Wayprompt requires the compositor to support the layer shell protocol, and GNOME doesn’t.

Flameopathic · 2025-05-12T12:47:25Z

Ah, the reason is that Wayprompt requires the compositor to support the layer shell protocol, and GNOME doesn’t.

That must be the same reason notification daemons won't work on it

trueNAHO

However, I agree with danth's stance of this abstraction being overkill. The best solution would be to replace GNOME with something like cage and make everything work there.

Instead of #1192 changing the entire graphical environment, it would be better if it would somehow be able to replace GNOME's notification daemon:

https://github.com/danth/stylix/blob/b66b220fb02cd0713eb93d82dcf36962d04b0d86/modules/mako/testbeds/default.nix#L4

I forgot to mention that I am fine with merging this PR for practical reasons, especially since using cage in our testbeds has been a long standing task that may involve nasty bugs and take longer than expected. In the meantime, more desktops can be supported, simplifying future incremental migrations to cage.

Also, it might be better to to refer to DEs and WMs as "graphical environment" instead of just "desktop".

+1 wayprompt doesn't work in gnome environment for some reason, i tested it on sway and hyprland, works great on those wms

Ah, the reason is that Wayprompt requires the compositor to support the layer shell protocol, and GNOME doesn’t.

Thanks for looking into this.

mateusauler · 2025-05-14T02:01:30Z

I'm currently unhappy with:

Each graphical environment module has to check if it should be enabled. (See stylix: allow choosing testbed desktop #1222 (comment))
The code for autostarting applications in WMs that don't support it is duplicated (See hyprland and bspwm)
The choice of available environments is somewhat arbitrary (they're the environments we have testbeds for). I think (Gnome ^ Plasma6) & (Hyprland ^ Bspwm) should be enough. Maybe cage as well.

Despite that, I believe this PR is ready for a proper review. So, I'm marking this as ready.

trueNAHO

LGTM, except for very minor things.

0xda157

LGTM aside from nit + naho's comments

This adds `stylix.testbed.ui.desktop` which allows the testbed to pick what desktop environment/window manager will be ran inside the VM.

stylix-automation · 2025-07-16T13:43:17Z

Backport failed for release-25.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-25.05
git worktree add -d .worktree/backport-1222-to-release-25.05 origin/release-25.05
cd .worktree/backport-1222-to-release-25.05
git switch --create backport-1222-to-release-25.05
git cherry-pick -x 8c854fe383fda20e8befefc31ecf248988a40bcc

Allow choosing the testbed desktop, ideally as a temporary solution until migrating to the cage environment. Link: nix-community#1222 Reviewed-by: Flameopathic <64027365+Flameopathic@users.noreply.github.com> Reviewed-by: awwpotato <awwpotato@voidq.com> Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com> (cherry picked from commit 8c854fe)

Fixes: 8c854fe ("stylix: allow choosing testbed desktop (#1222)") Link: #1714 Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com>

Allow choosing the testbed desktop, ideally as a temporary solution until migrating to the cage environment. Link: #1222 Reviewed-by: Flameopathic <64027365+Flameopathic@users.noreply.github.com> Reviewed-by: awwpotato <awwpotato@voidq.com> Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com> (cherry picked from commit 8c854fe)

Fixes: 8c854fe ("stylix: allow choosing testbed desktop (#1222)") Link: #1714 Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com> (cherry picked from commit 03699ed)

mateusauler force-pushed the push-xozvzyszlslx branch 6 times, most recently from fff4795 to c32bab3 Compare May 4, 2025 21:14

mateusauler changed the title ~~testbed: allow choosing desktop~~ stylix: allow choosing desktop May 4, 2025

mateusauler changed the title ~~stylix: allow choosing desktop~~ stylix: allow choosing testbed desktop May 4, 2025

mateusauler force-pushed the push-xozvzyszlslx branch from c32bab3 to af9721a Compare May 4, 2025 22:26

Flameopathic approved these changes May 4, 2025

View reviewed changes

mateusauler force-pushed the push-xozvzyszlslx branch from af9721a to 046be3a Compare May 6, 2025 01:28

mateusauler mentioned this pull request May 11, 2025

mako: add testbed #1192

Merged

5 tasks

trueNAHO requested changes May 11, 2025

View reviewed changes

0xda157 mentioned this pull request May 11, 2025

wayprompt: init #1250

Merged

5 tasks

mateusauler force-pushed the push-xozvzyszlslx branch from 046be3a to 9af04ef Compare May 11, 2025 23:48

trueNAHO requested changes May 12, 2025

View reviewed changes

Comment thread modules/bspwm/testbeds/default.nix Outdated

Comment thread stylix/testbed/default.nix Outdated

mateusauler force-pushed the push-xozvzyszlslx branch 3 times, most recently from e94573c to d00c805 Compare May 14, 2025 01:52

mateusauler marked this pull request as ready for review May 14, 2025 02:02

mateusauler force-pushed the push-xozvzyszlslx branch from d00c805 to 0ea01a5 Compare May 17, 2025 13:27

0xda157 reviewed May 17, 2025

View reviewed changes

Comment thread modules/bspwm/testbeds/default.nix Outdated

Comment thread stylix/testbed/graphicalEnvironments/bspwm.nix

mateusauler force-pushed the push-xozvzyszlslx branch from 0ea01a5 to 21a1e03 Compare May 17, 2025 19:57

trueNAHO requested changes Jul 12, 2025

View reviewed changes

mateusauler force-pushed the push-xozvzyszlslx branch from 5a1eebf to 0c5c1f7 Compare July 12, 2025 20:30

stylix-automation bot added status: merge conflict Merge conflict and removed status: merge conflict Merge conflict labels Jul 12, 2025

mateusauler force-pushed the push-xozvzyszlslx branch from 0c5c1f7 to 2d7585d Compare July 15, 2025 02:52

trueNAHO requested changes Jul 15, 2025

View reviewed changes

Comment thread stylix/testbed/graphicalEnvironments/hyprland.nix Outdated

Comment thread stylix/testbed/modules/application.nix Outdated

Comment thread stylix/testbed/default.nix Outdated

trueNAHO requested a review from 0xda157 July 15, 2025 10:29

trueNAHO requested changes Jul 15, 2025

View reviewed changes

Comment thread stylix/testbed/default.nix Outdated

stylix-automation bot removed the status: merge conflict Merge conflict label Jul 15, 2025

0xda157 reviewed Jul 15, 2025

View reviewed changes

Comment thread stylix/testbed/modules/application.nix Outdated

mateusauler force-pushed the push-xozvzyszlslx branch from 2d7585d to 342b2b5 Compare July 15, 2025 22:08

stylix: allow choosing testbed desktop

7f106f0

This adds `stylix.testbed.ui.desktop` which allows the testbed to pick what desktop environment/window manager will be ran inside the VM.

mateusauler force-pushed the push-xozvzyszlslx branch from 342b2b5 to 7f106f0 Compare July 15, 2025 22:12

trueNAHO requested changes Jul 16, 2025

View reviewed changes

Comment thread stylix/testbed/available-graphical-environments.nix Outdated

stylix/testbed/available-graphical-environments: remove blank line

ea8ff63

trueNAHO approved these changes Jul 16, 2025

View reviewed changes

trueNAHO merged commit 8c854fe into nix-community:master Jul 16, 2025
4 of 5 checks passed

mateusauler deleted the push-xozvzyszlslx branch July 16, 2025 13:45

nukdokplex mentioned this pull request Jul 16, 2025

wayprompt: add testbed #1708

Merged

5 tasks

trueNAHO mentioned this pull request Jul 16, 2025

[release-25.05] wayprompt: add testbed #1710

Merged

0xda157 mentioned this pull request Jul 17, 2025

[release-25.05] stylix: allow choosing testbed desktop #1713

Merged

5 tasks

0xda157 added the has: port to stable This has been backported to the latest stable branch label Jul 17, 2025

trueNAHO mentioned this pull request Jul 17, 2025

[release-25.05] waybar: add testbed #1715

Merged

trueNAHO mentioned this pull request Aug 29, 2025

i3: add testbed #1767

Open

5 tasks

Uh oh!

Conversation

mateusauler commented May 4, 2025

Things done

Notify maintainers

Uh oh!

danth commented May 4, 2025

Uh oh!

0xda157 commented May 4, 2025

Uh oh!

mateusauler commented May 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Flameopathic left a comment

Choose a reason for hiding this comment

Uh oh!

trueNAHO left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nukdokplex commented May 11, 2025

Uh oh!

panchoh commented May 12, 2025

Uh oh!

Flameopathic commented May 12, 2025

Uh oh!

trueNAHO left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mateusauler commented May 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

trueNAHO left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xda157 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stylix-automation bot commented Jul 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mateusauler commented May 4, 2025 •

edited

Loading